build: share example routes with web components - #802
Conversation
There was a problem hiding this comment.
Pull request overview
Aligns the web-components entrypoint with the Angular app’s shared route configuration so examples are loaded from the same lazy route map and route paths follow the component-selector naming convention (without the -demo suffix).
Changes:
- Switched the main app bootstrap to consume routes from
app.routes.ts. - Added a redirect from the empty path to
fluid-row-heightand renamed the “10k Rows” route toten-k-rows. - Refactored web-components example loading to resolve components from the shared
routesmap instead of a dedicated switch.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/main.web-components.ts | Builds an example loader map from shared routes and registers web component examples dynamically. |
| src/main.ts | Updates route import to use ./app/app.routes. |
| src/app/app.routes.ts | Adds default redirect and renames 10k-rows to ten-k-rows in the route map. |
| src/app/app.component.html | Updates navigation link to the renamed ten-k-rows route. |
Suppressed comments (1)
src/app/app.routes.ts:21
- This renames the example route from
10k-rowstoten-k-rows, which will break existing deep links/bookmarks (and any external references) to#/10k-rows. Consider keeping backward compatibility by adding a redirect route from the old path to the new one.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
927f3bc to
743676b
Compare
fh1ch
left a comment
fh1ch
left a comment
There was a problem hiding this comment.
@spike-rabbit nice work here, I like the direction 🙇
Just two points from my end, but the rest looks good 🏓
| // Basic | ||
| { | ||
| path: '10k-rows', | ||
| path: 'ten-k-rows', |
There was a problem hiding this comment.
IMO, this change doesn't make much sense. Everything is called 10k-rows (file, title, ...), so why do we suddenly call it ten-k? We should align this here, no hard feelings into which direction.
| @@ -1,15 +1,21 @@ | |||
| import { Routes } from '@angular/router'; | |||
|
|
|||
| // Route paths must match their component selectors without the `-demo` suffix. | |||
There was a problem hiding this comment.
Do we have some sort of linting for this? We don't really test our demo app, meaning errors will only be noticed during runtime/by the reader.
There was a problem hiding this comment.
My plan is to generate this file in the future. I guess this is easier than to lint it?
There was a problem hiding this comment.
@spike-rabbit agree, this is now also in place with #809.
What kind of change does this PR introduce? (check one with "x")
What is the current behavior? (You can also link to an open issue here)
Web-component examples are registered through a separate switch statement and do not share the application route configuration.
What is the new behavior?
Web-component examples load their components from the shared lazy route map. Route paths now follow the component selector convention without the
-demosuffix.Does this PR introduce a breaking change? (check one with "x")
If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...
Other information: